Skip to content

Fix fatal config set error - #13535

Open
sepehr-rs wants to merge 23 commits into
pypa:mainfrom
sepehr-rs:fix-config-error
Open

Fix fatal config set error#13535
sepehr-rs wants to merge 23 commits into
pypa:mainfrom
sepehr-rs:fix-config-error

Conversation

@sepehr-rs

Copy link
Copy Markdown
Member

Fixes #13279

I tried fixing this by providing the user with a helpful error message instead of crashing, and made a new parser to allow writing to the PIP_CONFIG_FILE environment variable temporarily.

One important caveat is that changes made to the config file via PIP_CONFIG_FILE may not be reflected in other commands like pip config get due to how pip loads config scopes.

Would appreciate any feedback on this :).

@sepehr-rs sepehr-rs changed the title Fix config error Fix fatal config set error Aug 11, 2025
@sepehr-rs

Copy link
Copy Markdown
Member Author

I think pre-commit is showing weird behavior here again...
I've changed the problematic files but it is still failing with the old files.

@notatallshaw

notatallshaw commented Aug 12, 2025

Copy link
Copy Markdown
Member

I think pre-commit is showing weird behavior here again...
I've changed the problematic files but it is still failing with the old files.

Do you know if there is anything particular about your git workflow? It does seem to be happening with your PRs more than anyone else's (though I have seen it happen at least once to mine).

You could perhaps report this to pre-commit, I would normally assist in reporting a bug but my experience with that project means I am not inclined to help it out.

In the mean time I'm thinking we should move off pre-commit.ci, and instead just add it as a github workflow, I don't see any advantage to having it run in a separate infrastructure. I'll make an issue and a PR when I have a moment.

@sepehr-rs

Copy link
Copy Markdown
Member Author

I think pre-commit is showing weird behavior here again...
I've changed the problematic files but it is still failing with the old files.

Do you know if there is anything particular about your git workflow? It does seem to be happening with your PRs more than anyone else's (though I have seen it happen at least once to mine).

You could perhaps report this to pre-commit, I would normally assist in reporting a bug but my experience with that project means I am not inclined to help it out.

In the mean time I'm thinking we should move off pre-commit.ci, and instead just add it as a github workflow, I don't see any advantage to having it run in a separate infrastructure. I'll make an issue and a PR when I have a moment.

No, I don’t think the issue is on my end. It might be that committing too quickly causes these problems. And sure, I’ll report this as a potential bug to pre-commit. Thanks for your feedback!

@notatallshaw

Copy link
Copy Markdown
Member

Oh great, Windows CI is failing today 😞 , I'll take a look in a min.

@notatallshaw

Copy link
Copy Markdown
Member

Seems like it was a transient error, I'll see if I can add some retry logic on the installing installing SVN step in the future.

Comment thread src/pip/_internal/configuration.py Outdated
@ichard26
ichard26 self-requested a review October 28, 2025 19:32
@notatallshaw

Copy link
Copy Markdown
Member

I don't think this PR fully addresses all the review comments yet, I will try and take a look in a few weeks.

@sepehr-rs

Copy link
Copy Markdown
Member Author

Gentle ping. I believe this is ready for review. This PR also addresses #12706, #11880, and #12481. I'd appreciate any feedback when someone has time.

@notatallshaw

Copy link
Copy Markdown
Member

Thanks for the ping, I'm going to be away on vacation for a week, but I hope some time in June to sit down and do make a big dent in the backlog of things I've promised to review.

@notatallshaw notatallshaw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The except IndexError in get_file_to_edit is now dead, the fallback returns or raises ConfigurationError.

Would be good to have some tests for unset + PIP_CONFIG_FILE, or some unit or functional tests in general.

Comment thread src/pip/_internal/configuration.py
Comment thread src/pip/_internal/configuration.py
Comment thread src/pip/_internal/configuration.py Outdated
Comment thread src/pip/_internal/configuration.py Outdated
@sepehr-rs

Copy link
Copy Markdown
Member Author

Thanks so much for the thorough reviews, @notatallshaw!

I've addressed the remaining points, got unset working, made the code raise an exception when PIP_CONFIG_FILE is set to devnull, and added tests for both.

I'd appreciate another look when you have a chance!

@sepehr-rs

Copy link
Copy Markdown
Member Author

Hey @ichard26, would you be open to considering adding this to the upcoming release milestone? I believe the latest review comments have been addressed and the requested tests have been added. No worries if it's too late in the release cycle or you'd prefer to leave it for a later release.

@ichard26 ichard26 added this to the 26.3 milestone Jul 25, 2026
@ichard26

Copy link
Copy Markdown
Member

TBH, pip's configuration code is one of my least favourite parts of the codebase to make changes. It's a mess to follow1 while being an easy source of backwards compatibility potholes. I'd need to invest a significant amount of time to understand what's the underlying issue here and whether this change is the right solution.

So, I consider this PR out of scope for the upcoming 26.2 release. I'll tack to the 26.3 milestone for the time being.

Footnotes

  1. Not critiquing the code itself or those who wrote any of it, but it's impenetrably hard to read

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pip config set ERROR: Fatal Internal error [id=2]. Please report as a bug.

4 participants